Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(papercut) - rerendered submenu actions for time sensitive actions #41670

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

emoral435
Copy link
Contributor

Summary

Changes submenu loading to re-render the actions in the submenu, as the items could potentially be time dependent. As of now, the items within the submenu are only loaded when dependencies change, or upon initial loading of the page, but by loading it whenever the user clicked the button, they wont have to reload the page, saving them an extra click ✅

BEFORE

piM9LcEWjP

AFTER

jFihBodbD6

TODO

  • Since the presence of the action should be evaluated ideally on click instead of on script execution I think the FileAction enabled callback needs to be set to something like () => Boolean(getDateTime(option.dateTimePreset)) to be more reactive
  • Made it so that when an action within the dropdown changes the openedSubmenu dependency, reloads only the submenu action and doesn't change other computed dependencies, which would slow down the loading of the page

Checklist

@emoral435 emoral435 requested a review from skjnldsv November 22, 2023 17:49
@emoral435
Copy link
Contributor Author

will compile assets once reviews look good!

@emoral435 emoral435 requested review from susnux and Pytal November 22, 2023 17:50
@AndyScherzinger AndyScherzinger requested a review from a team November 22, 2023 18:29
@blizzz blizzz added this to the Nextcloud 29 milestone Nov 23, 2023
@blizzz blizzz added the 3. to review Waiting for reviews label Nov 23, 2023
@emoral435 emoral435 enabled auto-merge November 27, 2023 23:52
@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from 97ffc66 to 488ded0 Compare November 28, 2023 00:02
@emoral435 emoral435 added enhancement papercut Annoying recurring issue with possibly simple fix. feature: activity and notification Nice to have labels Nov 28, 2023
@emoral435 emoral435 requested a review from Pytal November 28, 2023 00:53
@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from 468c71b to 048e8e2 Compare November 28, 2023 01:28
susnux
susnux previously requested changes Nov 28, 2023
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is 3rdparty changed?

Copy link
Member

@Pytal Pytal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just gotta remove 3rdparty changes 🙈

@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from 048e8e2 to 1be3445 Compare November 28, 2023 15:20
@emoral435
Copy link
Contributor Author

/compile amend/

@emoral435 emoral435 dismissed susnux’s stale review November 28, 2023 15:21

fixed 3rdparty folder - checked out the correct branch for 3rdparty submodule

@emoral435 emoral435 requested a review from susnux November 28, 2023 15:21
@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from fcbbd30 to 379c58a Compare November 29, 2023 23:32
@emoral435 emoral435 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 29, 2023
@emoral435
Copy link
Contributor Author

/compile amend /

@nextcloud-command nextcloud-command force-pushed the fix/reminder_conditional_rendering branch from 379c58a to f313d3f Compare November 29, 2023 23:58
@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from f313d3f to e1788f2 Compare November 30, 2023 01:01
@Pytal
Copy link
Member

Pytal commented Nov 30, 2023

Let's squash these commits into one @emoral435 :)

@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from e1788f2 to b7a00d8 Compare November 30, 2023 01:50
@emoral435 emoral435 disabled auto-merge November 30, 2023 15:32
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove the last .filter(Boolean) as FileAction[] then :)

@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from b7a00d8 to 6b95c6d Compare December 8, 2023 17:34
@emoral435
Copy link
Contributor Author

/compile amend /

@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch 2 times, most recently from a485b3a to c31352d Compare December 9, 2023 03:19
@emoral435 emoral435 force-pushed the fix/reminder_conditional_rendering branch from c31352d to c4f6803 Compare December 9, 2023 03:20
@emoral435
Copy link
Contributor Author

/compile /

Signed-off-by: nextcloud-command <[email protected]>
@emoral435 emoral435 requested a review from skjnldsv December 11, 2023 16:59
@skjnldsv skjnldsv merged commit 3c704d4 into master Dec 12, 2023
42 checks passed
@skjnldsv skjnldsv deleted the fix/reminder_conditional_rendering branch December 12, 2023 07:34
@skjnldsv
Copy link
Member

/backport to stable28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish backport-request enhancement feature: activity and notification Nice to have papercut Annoying recurring issue with possibly simple fix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move reminder conditional rendering into action enabled
6 participants